Porting tests from archived repository of OpenPAYGO-Token #27
Conversation
dmohns
left a comment
dmohns
left a comment
There was a problem hiding this comment.
Looks really good already! Left a couple of comments
| Create new issues in the | ||
| [issue tracker](https://github.com/EnAccess/OpenPAYGO-python/issues/new/choose) |
There was a problem hiding this comment.
Please don't change this file. It's centrally managed for all our repositories 😉
There was a problem hiding this comment.
ahh I was getting linting error here for md I believe or smthg that's why had to change that as tests were failing but will see how I can fix it
| if isinstance(secret_key, (bytes, bytearray)): | ||
| secret_key_bytes = bytes(secret_key) | ||
| if len(secret_key_bytes) != 16: | ||
| raise ValueError( | ||
| "The secret key provided is not correctly formatted, it should be " | ||
| "16 " | ||
| "bytes. " | ||
| ) | ||
| return secret_key_bytes | ||
|
|
There was a problem hiding this comment.
Why do you need to change code in the metrics part? Your changes are only touching Token tests, right?
| if not past_used_counts: | ||
| if past_used_counts is None: |
There was a problem hiding this comment.
Why is this change required? It seem less "pythonic". Looks like you want to continue processing, when past_used_counts is an empty array, but exit the processing when None. Can you find a more descriptive way of achieving this?
| from .device_simulator import DeviceSimulator as DeviceSimulator | ||
| from .server_simulator import SingleDeviceServerSimulator as SingleDeviceServerSimulator | ||
|
|
||
| __all__ = ["DeviceSimulator", "SingleDeviceServerSimulator"] |
There was a problem hiding this comment.
I don't think these "Simulators" make sense as part of the core module. I know the legacy repository had it that way, but it wasn't ported to OpenPAYGO-python.
Could you move the simulators into the tests directory? As this is the only place where it's (meaningfully) used.
|
Hey @CoderOMaster another comment: I have updated the library to use a more modern tooling Also, probably need to resolve merge conflicts. Apologies for the additional effort 🙏 |
3 participants
Brief summary of the change made
Copied tests cases from OpenPAYGO-Token Repository (Archived), refactored them based on relevant imports, added error handling during hex decoding, copied and refactor device and service simulators and improved token encoding and decoding script to handle changes.
Link-> https://github.com/EnAccess/OpenPAYGO-Token.git
closes: #13
Are there any other side effects of this change that we should be aware of?
I do not think so
Describe how you tested your changes?
I have followed the readme.md to run my changes as well as pytest for original test to make sure it works
Pull Request checklist
Please confirm you have completed any of the necessary steps below.